api: adds hits_addend specifier to RateLimitRule #4957
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4957 +/- ##
=======================================
Coverage 66.71% 66.71%
=======================================
Files 209 209
Lines 32052 32052
=======================================
Hits 21384 21384
- Misses 9388 9390 +2
+ Partials 1280 1278 -2 ☔ View full report in Codecov by Sentry. |
|
feel like maybe calling it "weight" instead of usage maybe be better? idk |
|
will add the cel validation this afternoon (in japan) |
|
maybe simply Usage->HitAddends would be consistent |
mathetake
changed the title
mathetake
changed the title
|
@zirain @zhaohuabing ptal - I removed the implementation-hide as envoyproxy/ratelimit#802 got merged |
|
i will do the implementation in a subsequent PR |
@mathetake the implementation comment is used to hide the unimplemented EG API in the EG docs, we normally hide the API at the API PR and unhide it at the implementation PR. |
|
ah ok - will do |
|
@zhaohuabing hid the APIs! |
|
👍 |
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
zirain
force-pushed
the
applyonstreamdone
branch
from
c125ff2 to
d1b3de3
Compare
| @@ -91,6 +92,64 @@ type RateLimitRule struct { | |||
| // 429 HTTP status code is sent back to the client when | |||
| // the selected requests have reached the limit. | |||
| Limit RateLimitValue `json:"limit"` | |||
| // RequestHitsAddend specifies the number to reduce the rate limit counters | |||
There was a problem hiding this comment.
thanks for introducing this @mathetake !
thinking out loud, and trying to describe user intent
until now, all we had was "I want to ratelimit at X requests per time unit", now we also have "I want to ratelimit requests based on a total count X, with each response adding to that count by a value (from a response header)"
Im unsure if cost is the best word to use here, hitsAddend is not intuitive enough imo, another option is count e.g. countPerRequest or perRequestCount or simply requestCount
costPerRequest: <val>. // defaults to 1
costPerResonse: // defaults to 0
from:
type: ResponseHeader / Header
.......
cc @envoyproxy/gateway-reviewers @envoyproxy/gateway-maintainers
There was a problem hiding this comment.
Im unsure if cost is the best word to use here, hitsAddend is not intuitive enough imo, another option is count e.g. countPerRequest or perRequestCount or simply requestCount
yeah agree not that intuitive, and costPer* sounds good to me
There was a problem hiding this comment.
btw, our primary usecase is not getting the cost from response header but from a dynamic metadata - so having the explicit from:type:.. won't be much helpful and I would rather expose the opaque "format" which supports both dynamic metadata and headers
There was a problem hiding this comment.
can we map to format internally, and somehow use a user facing construct instead ?
There was a problem hiding this comment.
gotcha - will work on it as well as renaming to costPer* stuff tomorrow or monday (in japan 😉)
|
btw currently the per-descriptor hits_addend is only supported for per-route ratelimit policy config either via virtual or route config according to envoyproxy/envoy#37684 cc @wbpcode is that an issue for EG? edited: nvm, i think it's not an issue |
yeah should be fine gateway/internal/xds/translator/ratelimit.go Line 141 in 10a31f1 |
What type of PR is this?
This adds a new API into rate limit API.
What this PR does / why we need it:
This is for #4756. Basically, this adds the API to specify the hits_addend for the rate limit rule.
Especially, configuring the hits_addend in the response_path allows us to "reduce" the counter
based on the response content that affects the subsequent requests. This will enable the "token"
based request limits that are required fro AI gateway.
This is based on the Envoy's brand new features called apply_on_stream_done
and per-descriptor level hits_addend configuration introduced in
Which issue(s) this PR fixes:
#4756
Release Notes: No (until the implementation is done)